Update encode_qr.py - Fixes #582 #610
Conversation
Implementing "use_info.network" member provided on HDKey/CoinInfo classes (urtypes/crypto packages-libs folder) for testnet public key exporting option.
|
#582 Exporting for testnet when option is selected. |
|
I couldn't replicate the failing test locally (using python 3.12), @newtonick @kdmukai could you re-run the ci? I suspect that test is maybe just flaky 
|
|
I can't reproduce the failed CI locally either 
-- |
|
I'm able to reproduce the failed CI locally now |
|
Thank you, @newtonick. What could you conclude? Any particular on the enviroment to take present for this PR? |
|
@newtonick could you comment on how were you able to reproduce the failing test? |
Change in "self.use_info" assignation for to be "None" when mainnet.
Updates first test (in test_ur_xpub_qr) to "MAINNET" and creates following for "TESTNET"
|
According to suggested by @newtonick, the commits (pytested on local) for implementing "use_info" only on testnet in order to avoid bloat the QR. Updating mainnet case and creating new one for testnet in test_encodepsbtqr.py which was been affected. #582 |
|
Very nice fedebuyito. ACK tested. |
src/seedsigner/models/encode_qr.py
Outdated
| @@ -344,11 +344,13 @@ def derivation_to_keypath(path: str) -> list: | |||
| return Keypath(arr, self.root.my_fingerprint, len(arr)) | |||
|
|
|||
| origin = derivation_to_keypath(self.derivation) | |||
| self.use_info = None if self.network == "M" else CoinInfo(type=None, network=1) | |||
There was a problem hiding this comment.
Should always use the constants instead of hard-coded strings.
self.use_info = None if self.network == SettingsConstants.MAINNET else...
src/seedsigner/models/encode_qr.py
Outdated
| @@ -344,11 +344,13 @@ def derivation_to_keypath(path: str) -> list: | |||
| return Keypath(arr, self.root.my_fingerprint, len(arr)) | |||
|
|
|||
| origin = derivation_to_keypath(self.derivation) | |||
| self.use_info = None if self.network == "M" else CoinInfo(type=None, network=1) | |||
There was a problem hiding this comment.
An explanatory comment would help here and/or a reference to Issue #582.
There was a problem hiding this comment.
You are right. Done on commit 0496eea bellow.
There was a problem hiding this comment.
@fedebuyito I meant in the code itself:
origin = derivation_to_keypath(self.derivation)
# For testnet we need to... blah blah... because... blah blah...
self.use_info = None if self.network == "M" else CoinInfo(type=None, network=1)It's not urgent, but since this PR is already merged, can you add that code comment and submit that change as its own PR?
|
If you update the PR description to include "fixes #582", Github will automatically mark that Issue as closed when the PR is merged. |
| @@ -344,7 +344,7 @@ def derivation_to_keypath(path: str) -> list: | |||
| return Keypath(arr, self.root.my_fingerprint, len(arr)) | |||
|
|
|||
| origin = derivation_to_keypath(self.derivation) | |||
| self.use_info = None if self.network == "M" else CoinInfo(type=None, network=1) | |||
| self.use_info = None if self.network == SettingsConstants.MAINNET else CoinInfo(type=None, network=1) | |||
There was a problem hiding this comment.
Implements "use_info.network" member provided on HDKey/CoinInfo classes (urtypes/crypto packages-libs folder) for enable testnet public key export option ("self.use_info" assignation for to be "None" when mainnet). #582
There was a problem hiding this comment.
Replaces "M" for setting constant relative to Mainnet.
|
ACK |
Implementing "use_info.network" member provided on HDKey/CoinInfo classes (urtypes/crypto packages-libs folder) for testnet public key exporting option.
Description
Describe the change simply. Provide a reason for the change.
Include screenshots of any new or modified screens (or at least explain why they were omitted)
This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before sumbitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os:
Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.